Conversation
Agent-Logs-Url: https://github.com/streamich/react-embed/sessions/03024238-5624-486d-bf5d-3fe9b9dd6a42 Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an oEmbed-based fallback block to broaden URL embed coverage (especially for providers without dedicated blocks) and exposes the oEmbed utilities as part of the public package API.
Changes:
- Introduces a new
oembedblock with a safe info-card renderer and an opt-in HTML renderer. - Adds provider detection + oEmbed fetch utility and exports them from the package root.
- Routes unknown URLs to the new
oembedblock as a final fallback and adds Storybook examples.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/stories/ReactEmbed.stories.tsx | Adds Storybook stories for the new oEmbed block modes. |
| src/routeToBlock.ts | Adds oEmbed routing as a final fallback after existing handlers. |
| src/index.tsx | Exports oEmbed utilities and types from the package root. |
| src/blocks/oembed/index.tsx | Implements the React oEmbed block (card + opt-in HTML injection). |
| src/blocks/oembed/fetchOEmbed.ts | Adds provider detection and oEmbed JSON fetch utility. |
| src/ReactEmbed.tsx | Registers the new oembed block in defaultBlocks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function getOEmbedEndpoint(url: string): string | null { | ||
| for (const provider of PROVIDERS) { | ||
| if (provider.patterns.some((p) => p.test(url))) { | ||
| return `${provider.endpoint}?url=${encodeURIComponent(url)}&format=json`; | ||
| } | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Provider detection uses regexes against the full URL string (e.g. /youtube\.com/), which can false-positive on unrelated URLs that merely contain that substring (path/query) and may leak arbitrary URLs to third-party oEmbed endpoints. Parse with new URL(url) and match against hostname (or anchor the regex to the domain) to ensure only real provider domains are matched.
| export async function fetchOEmbed(url: string, signal?: AbortSignal): Promise<OEmbedResponse> { | ||
| const endpoint = getOEmbedEndpoint(url); | ||
| if (!endpoint) { | ||
| throw new Error(`No oEmbed provider found for URL: ${url}`); | ||
| } | ||
| const response = await fetch(endpoint, {signal}); | ||
| if (!response.ok) { | ||
| throw new Error(`oEmbed request failed with status ${response.status}`); | ||
| } | ||
| return response.json() as Promise<OEmbedResponse>; |
There was a problem hiding this comment.
fetchOEmbed relies on a global fetch being available. Since this utility is exported from the package root and can be used outside React/browser contexts, it would be safer to either (a) document that callers must provide a fetch polyfill, or (b) accept an injected fetch implementation via an optional parameter to avoid runtime failures in Node/older environments.
| export const OEmbedCard = { | ||
| args: { | ||
| url: oEmbedUrls[0], | ||
| }, | ||
| argTypes: { | ||
| url: { | ||
| options: oEmbedUrls, | ||
| control: {type: 'select'}, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| export const OEmbedHtml = { | ||
| args: { | ||
| url: oEmbedUrls[0], | ||
| blocks: {oembed: createOEmbedBlock({renderHtml: true})}, | ||
| }, |
There was a problem hiding this comment.
The OEmbedCard/OEmbedHtml stories default to oEmbedUrls[0] which is a Vimeo URL. routeToBlock routes Vimeo to reactPlayer via canPlay(url) before the oEmbed fallback, so these stories may not actually exercise the new oEmbed block by default. Consider defaulting to a URL that routes to oembed (e.g. Spotify/Flickr/CodePen) or explicitly overriding routing for the story.
|
|
||
| const thumbnail = | ||
| data.type === 'photo' ? ( | ||
| <img src={data.url || data.thumbnail_url} alt={data.title || ''} style={thumbnailStyle} /> |
There was a problem hiding this comment.
In the photo branch, the thumbnail always renders even if both data.url and data.thumbnail_url are missing, producing an <img> without a src (React will omit the attribute). This can cause unexpected browser behavior (e.g. requesting the current document). Add a guard so the <img> only renders when a usable URL is present.
| const thumbnail = | |
| data.type === 'photo' ? ( | |
| <img src={data.url || data.thumbnail_url} alt={data.title || ''} style={thumbnailStyle} /> | |
| const photoSrc = data.url || data.thumbnail_url; | |
| const thumbnail = | |
| data.type === 'photo' ? ( | |
| photoSrc ? <img src={photoSrc} alt={data.title || ''} style={thumbnailStyle} /> : null |
| componentDidMount() { | ||
| const {signal} = this.controller; | ||
| fetchOEmbed(this.props.url, signal) | ||
| .then((data) => { | ||
| if (!signal.aborted) this.setState({data, loading: false}); | ||
| }) | ||
| .catch((error) => { | ||
| if (!signal.aborted) this.setState({error, loading: false}); | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
OEmbedBlock only fetches in componentDidMount, so if the parent updates url (e.g. Storybook select / consumer changing props), the component will keep showing the previous oEmbed result. Add componentDidUpdate to detect this.props.url changes, abort any in-flight request, reset state to loading, and refetch for the new URL (or force a remount via a key).
| componentDidMount() { | |
| const {signal} = this.controller; | |
| fetchOEmbed(this.props.url, signal) | |
| .then((data) => { | |
| if (!signal.aborted) this.setState({data, loading: false}); | |
| }) | |
| .catch((error) => { | |
| if (!signal.aborted) this.setState({error, loading: false}); | |
| }); | |
| } | |
| fetchData() { | |
| const {signal} = this.controller; | |
| fetchOEmbed(this.props.url, signal) | |
| .then((data) => { | |
| if (!signal.aborted) this.setState({data, loading: false, error: null}); | |
| }) | |
| .catch((error) => { | |
| if (!signal.aborted) this.setState({data: null, error, loading: false}); | |
| }); | |
| } | |
| componentDidMount() { | |
| this.fetchData(); | |
| } | |
| componentDidUpdate(prevProps: BlockProps & {renderHtml: boolean}) { | |
| if (prevProps.url !== this.props.url) { | |
| this.controller.abort(); | |
| this.controller = new AbortController(); | |
| this.setState({data: null, loading: true, error: null}, () => { | |
| this.fetchData(); | |
| }); | |
| } | |
| } |
| <p style={titleStyle}> | ||
| {data.author_url ? ( | ||
| <a href={data.author_url} target="_blank" rel="noopener noreferrer" style={anchorStyle}> | ||
| {data.title} | ||
| </a> | ||
| ) : ( | ||
| data.title | ||
| )} | ||
| </p> |
There was a problem hiding this comment.
The title link uses data.author_url, which (per oEmbed) is the author/profile URL, not the resource URL. This will send users to the author page when clicking the embed title. Consider linking the title to the original props.url (or data.url for photo type) and keep author_url only for the author line.
| if (renderHtml && data.html) { | ||
| // Opt-in: inject the provider's own HTML (e.g. an <iframe>) | ||
| return renderWrap(<div dangerouslySetInnerHTML={{__html: data.html}} />); | ||
| } |
There was a problem hiding this comment.
dangerouslySetInnerHTML injects provider-supplied HTML without any sanitization. Even if this is opt-in, it creates an easy XSS footgun when consumers embed user-provided URLs. Consider adding a sanitization/validation hook (e.g. option to transform/sanitize data.html) and/or additional guardrails/documentation in the API to make the risk explicit.
type Provider = [patterns: string[], endpoint: string];
|
…obing Agent-Logs-Url: https://github.com/streamich/react-embed/sessions/d8e7c380-2dda-4223-9240-b18011beb43d Co-authored-by: streamich <9773803+streamich@users.noreply.github.com>
Done in 75a00a8:
|
Implements the oEmbed open standard to render embeds for providers not handled by dedicated blocks (Flickr, Reddit, TikTok, CodePen, Dailymotion, Giphy, Spotify, Slideshare, Speakerdeck, etc.) and as a richer fallback path.
New:
src/blocks/oembed/fetchOEmbed.ts— pure fetch utility, no React dependency:getOEmbedEndpoint(url): string | null— maps a page URL to its provider's JSON endpoint; matching is done against the parsed hostname (no false positives from substrings in paths/queries)fetchOEmbed(url, signal?): Promise<OEmbedResponse>— fetches typed oEmbed metadata; acceptsAbortSignalProvider2-tuple ([hostnames: string[], endpoint: string]): YouTube, Vimeo, Twitter/X, SoundCloud, Instagram, Flickr, Spotify, Reddit, TikTok, CodePen, Dailymotion, Giphy, Slideshare, Speakerdeck, Mixcloud, DeviantArt, TED, Tumblr, Kickstarter, Meetup, 500px, Twitch, Apple Music, Pinterest, Deezer, Bandcamp, Wistia, Loom, CanvafetchOEmbedsequentially probes{origin}/oembed,/oembed.json,/api/oembed,/services/oembedbefore giving upindex.tsx— React block:createOEmbedBlock({ renderHtml: true })injects the provider'shtmlfield viadangerouslySetInnerHTML(caller accepts XSS risk)AbortControllerto cancel in-flight fetches on unmount orurlprop changecomponentDidUpdatewhenurlprop changesRouting & registration
routeToBlock.tsusesgetOEmbedEndpointas the final fallback afterpdf/simplePlayer/reactPlayer. Theoembedblock is added todefaultBlocksinReactEmbed.tsx.Usage
fetchOEmbed,getOEmbedEndpoint,createOEmbedBlock,OEmbedResponse,OEmbedBlockOptions, andProviderare exported from the package root.